Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Anvil Chunk saving #401

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Anvil Chunk saving #401

wants to merge 7 commits into from

Conversation

Snowiiii
Copy link
Member

@Snowiiii Snowiiii commented Dec 18, 2024

Description

Add Chunk saving via the Anvil format.
More details on how the formats works can be found at https://minecraft.fandom.com/wiki/Region_file_format

Testing

  • Test Saving Chunks
  • Test Loading Saved Chunks (Pumpkin)
  • Test Loading Saved Chunks (Client)

Checklist

Things need to be done before this Pull Request can be merged.

  • Write header (size & compression scheme)

Please follow our Coding Guidelines

Copy link
Contributor

@kralverde kralverde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some reviews to the code. Would like to see some unit tests too, basically create a fake region with (1, 2 ... all) chunks getting serialized. Then immediately de-serialize and verify all data is the same. With all compressions also.

@@ -141,41 +186,300 @@ impl ChunkReader for AnvilChunkReader {
out
};

// TODO: check checksum to make sure chunk is not corrupted
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was a checksum implemented?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not that i know, this was lucas comment

)
});

region_file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect, the pad should make the whole file a multiple of 4096, not just add 4096. It should be len(header) + len(bytes) + len(pad) % 4096 == 0

}

sections.push(ChunkSection {
y: i as i8,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There can be -y heights, but i is a usize. Should there be a start_y passed in?

pumpkin-world/src/chunk/mod.rs Outdated Show resolved Hide resolved
@@ -189,16 +176,22 @@ impl Level {
self.chunk_watchers.shrink_to_fit();
}

pub fn write_chunk(&self, _chunk_to_write: (Vector2<i32>, Arc<RwLock<ChunkData>>)) {
//TODO
pub async fn write_chunk(&self, chunk_to_write: (Vector2<i32>, Arc<RwLock<ChunkData>>)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need some kind of file lock so multiple threads don't try to write to the same file at the same time/read while the file is being written

@Snowiiii
Copy link
Member Author

For some reason currently loading vanilla chunks does not work when writing, when early return in the write_chunk then it works

@Snowiiii Snowiiii marked this pull request as draft December 30, 2024 10:56
@Snowiiii
Copy link
Member Author

Snowiiii commented Jan 3, 2025

Looking for help, Mojang did a great job making this Format really complicated. If someone wants to help i would really appreciate it.

@Snowiiii Snowiiii requested a review from kralverde January 3, 2025 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants